Skip to content

Add backend-aware test parameter scaling for pure Python CI #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
igerber wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add backend-aware test parameter scaling for pure Python CI #129

igerber wants to merge 1 commit into from

Conversation

@igerber
Copy link
Owner

@igerber igerber commented Feb 1, 2026

Summary

  • Add CIParams class and ci_params session-scoped pytest fixture in conftest.py that detects pure Python mode and scales down bootstrap iterations and TROP grid sizes
  • Update ~89 test functions across 7 test files to use ci_params.bootstrap(n) for bootstrap iteration counts and ci_params.grid(values) for TROP lambda grids
  • Make 11 hardcoded assertion values dynamic to match scaled parameters
  • Leave API-only tests (no .fit() calls) and warning-threshold tests unchanged

Methodology references (required if estimator / math changes)

  • N/A - no methodology changes. Only test parameter scaling; same code paths exercised with fewer iterations in pure Python mode.

Validation

  • Tests added/updated: tests/conftest.py, tests/test_trop.py, tests/test_staggered.py, tests/test_wild_bootstrap.py, tests/test_sun_abraham.py, tests/test_estimators.py, tests/test_methodology_callaway.py, tests/test_methodology_did.py
  • All 387+ tests across modified files pass with Rust backend
  • With Rust backend, all values pass through unchanged (zero behavior change)

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

@igerber @claude
Add backend-aware test parameter scaling for pure Python CI performance
1f5127e 
Introduce ci_params session-scoped fixture that scales bootstrap iterations
and TROP grid sizes when running without Rust backend, reducing CI time
while preserving code path coverage.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Feb 1, 2026

Overall assessment: ⚠️ Needs changes

Executive summary:

  • P1: The TROP paper-conformance test now accepts as few as 11 successful bootstrap draws even when n_boot=30, weakening the regression guard for stratified sampling.
  • P2: Bootstrap scaling in pure Python mode reduces methodology convergence tests to ~35 iterations, which can make SE-convergence assertions noisy and less meaningful.
  • No estimator or inference logic changes detected in this diff; methodology registry requirements appear unchanged.
  • AGENTS.md not found in repo; proceeded without additional agent instructions.

Methodology

  • Severity: P1 | Impact: The paper-conformance test for stratified bootstrap now allows as few as 11 successful draws even when n_boot=30, which weakens detection of regressions in Algorithm 3’s stratified sampling behavior. | Concrete fix: Make the threshold proportional to n_boot (e.g., >= max(11, int(0.67 * n_boot)) or >= n_boot - 5) so Rust-backend runs keep the stronger expectation while pure Python scaling remains feasible. tests/test_trop.py:L1615-L1669
  • Severity: P3 | Impact: No estimator/assumption/inference code paths were changed in this diff; registry requirements are unaffected. | Concrete fix: None. docs/methodology/REGISTRY.md:L1-L40

Code Quality

  • Severity: P3 | Impact: No code-quality issues identified in the changed test/fixture code beyond findings above. | Concrete fix: None.

Performance

  • Severity: P3 | Impact: Changes are test-only performance optimizations; no production performance impact observed. | Concrete fix: None.

Maintainability

  • Severity: P3 | Impact: No maintainability issues introduced beyond findings above. | Concrete fix: None.

Tech Debt

  • Severity: P3 | Impact: No new tech-debt concerns introduced in this diff. | Concrete fix: None.

Security

  • Severity: P3 | Impact: No security-sensitive changes or secrets observed in the diff. | Concrete fix: None.

Documentation/Tests

  • Severity: P2 | Impact: Methodology convergence tests now use ci_params.bootstrap(499), which scales to ~35 iterations in pure Python mode; this can make the 20–25% SE-convergence assertions noisy and reduce methodological verification strength. | Concrete fix: Exempt these convergence tests from scaling (keep high n_boot), or conditionally relax/skip when n_boot falls below a minimum threshold. tests/conftest.py:L99-L113, tests/test_methodology_callaway.py:L803-L839

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@igerber